Skip to content

fix: resolve event listener and timer cleanup leaks causing memory growth#832

Draft
notJoon wants to merge 1 commit intodevelopfrom
fix/event-listener-timer-leaks
Draft

fix: resolve event listener and timer cleanup leaks causing memory growth#832
notJoon wants to merge 1 commit intodevelopfrom
fix/event-listener-timer-leaks

Conversation

@notJoon
Copy link
Copy Markdown
Member

@notJoon notJoon commented Feb 25, 2026 •
edited by coderabbitai Bot
Loading

Summary

  • useModalCloseEvent: Cleanup called addEventListener instead of removeEventListener, accumulating keydown listeners on every unmount
  • SubMenuButton: Cleanup removed mouseover instead of mousemove, leaving dangling listeners on document across all pages
  • useAutoDisconnect: Cleanup called clearTimeout on a setInterval ID, preventing the 10s background check internal from ever being cleared

Changes

useModalCloseEvent

Before After
window.addEventListener("keydown", handleEsc) in cleanup window.removeEventListener("keydown", handleEsc)
Handler functions recreated every render (stale closure) Wrapped in useCallback with proper deps
useEffect deps: [] useEffect deps: [handleEsc, onClickOutbound]

SubMenuButton

Before After
removeEventListener("mouseover", ...) removeEventListener("mousemove", ...)

useAutoDisconnect

Before After
clearTimeout(backgroundCheckIntervalRef.current) clearInterval(backgroundCheckIntervalRef.current)

Performance Measurements

Tested by opening/closing a SelectBox dropdown 10 times and recording Chrome DevTools Performance for ~15 seconds.

Before (tested on beta.gnoswap.io)

수정 전

After (tested on Vercel preview)

수정 후

Results

Metric Before After Change
Listeners Staircase increase on every open/close Stable after initial mount Accumulation eliminated
JS Heap range 54.1 → 68.8 MB (+14.7 MB) 48.8 → 59.7 MB (+10.9 MB) ~26% less growth
Profiling overhead 85.6% of self time 46.9% of self time Reduced listener dispatch overhead

Summary by CodeRabbit

  • Bug Fixes
    • Corrected event listener cleanup in sub-menu interactions to properly remove registered listeners
    • Fixed timer cleanup to use the correct cancellation method
    • Improved modal close event handler stability and cleanup to prevent memory leaks and ensure proper event removal

@vercel
Copy link
Copy Markdown

vercel Bot commented Feb 25, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
gnoswap-interface Building Building Preview, Comment Feb 25, 2026 8:30am

Request Review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Feb 25, 2026 •
edited
Loading

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 89fa8c4 and 7d0108a.

📒 Files selected for processing (3)
  • packages/web/src/components/common/header/sub-menu-button/SubMenuButton.tsx
  • packages/web/src/hooks/common/use-auto-disconnect.tsx
  • packages/web/src/hooks/common/use-modal-close-event.tsx

Walkthrough

Three files fix event listener cleanup issues and improve hook dependency management. SubMenuButton corrects mouseover listener cleanup, use-auto-disconnect replaces clearTimeout with clearInterval, and use-modal-close-event introduces useCallback memoization and fixes listener cleanup logic.

Changes

Cohort / File(s) Summary
Event Listener Cleanup Fixes
packages/web/src/components/common/header/sub-menu-button/SubMenuButton.tsx, packages/web/src/hooks/common/use-auto-disconnect.tsx		 SubMenuButton aligns cleanup to remove mousemove listener matching registered event. use-auto-disconnect corrects cleanup to use clearInterval instead of clearTimeout for interval timers.
Modal Close Event Hook Refactor
packages/web/src/hooks/common/use-modal-close-event.tsx		 Introduces useCallback for memoized event handlers (handleEsc, onClickOutbound). Updates useEffect dependency array to [handleEsc, onClickOutbound]. Corrects cleanup to remove both click and keydown listeners, fixing duplicate keydown listener issue.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main objective: fixing event listener and timer cleanup leaks. All three changes in the PR address memory growth from improper cleanup, making the title directly relevant to the changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/event-listener-timer-leaks

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Feb 25, 2026

Quality Gate Passed Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@notJoon notJoon changed the title fix: fix event listener and interval cleanup bugs causing memory leaks fix: resolve event listener and timer cleanup leaks causing memory growth Feb 25, 2026
@notJoon notJoon marked this pull request as ready for review February 25, 2026 08:53
@notJoon notJoon marked this pull request as draft February 25, 2026 09:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@notJoon